Skip to content

Fix 1441 in 4_4_x - scope and collection not considered in repo delete. #1465

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

mikereiche
Copy link
Collaborator

This is a more localized change than the one in main.

Closes #1464.

  • You have read the Spring Data contribution guidelines.
  • There is a ticket in the bug tracker for the project in our JIRA.
  • You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
  • You submit test cases (unit or integration tests) that back your changes.
  • You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).

This is a more localized change than the one in main.

Closes #1464.
@mikereiche mikereiche requested a review from daschl June 16, 2022 22:22
@@ -60,7 +60,7 @@ public class ReactiveCouchbaseRepositoryKeyValueIntegrationTests extends Cluster

@Autowired ReactiveUserRepository userRepository;

@Autowired ReactiveAirportRepository airportRepository;
@Autowired ReactiveAirportRepository reactiveAirportRepository;

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to rename the ReactiveAiportRepository to exactly match the bean-name for disambiguation - as the ReactiveAirportRepositoryAnnotated is also a ReactiveAirportRepository.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍


@SpringJUnitConfig(Config.class)
@IgnoreWhen(missesCapabilities = { Capabilities.QUERY, Capabilities.COLLECTIONS }, clusterTypes = ClusterType.MOCKED)
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the missing @SpringJunitConfig was causing the @Autowired not to work, and requiring a hack in beforeAll()

Copy link
Contributor

@daschl daschl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple comments, but looks good overall.

this.findOperationWithProjection = findOp;
this.findOp = (ReactiveFindByQuery<?>) (operations.findByQuery(type).inScope(method.getScope())
.inCollection(method.getCollection()));
this.removeOp = (ReactiveRemoveByQuery<?>) (operations.removeByQuery(type).inScope(method.getScope())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can it be casted to findBy and RemoveBy at the same time?

@@ -65,6 +67,8 @@ public AbstractCouchbaseQuery(CouchbaseQueryMethod method, CouchbaseOperations o
ExecutableFindByQuery<?> findOp = operations.findByQuery(type);
findOp = (ExecutableFindByQuery<?>) (findOp.inScope(method.getScope()).inCollection(method.getCollection()));
this.findOperationWithProjection = findOp;
this.removeOp = (ExecutableRemoveByQuery<?>) (operations.removeByQuery(type).inScope(method.getScope())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as comment below, is it possible that not both casts are possible?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I understand the comment correctly - both are fine. the first one is operations.findByQuery(), the second is operations.removeByQuery(). I'm actually not sure why a cast is even required - because ExecutableRemoveByQuery (eventually) extends the type returned by withCollection() (RemoveByQueryWithOptions).

@@ -99,6 +101,15 @@
@ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
Mono<Airport> findByIata(String iata);

@Query("#{#n1ql.delete} WHERE #{#n1ql.filter} and iata = $1 #{#n1ql.returning}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: extra whitespace between and and iata, also below

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oof. Formatter does not reformat quoted strings!

@@ -60,7 +60,7 @@ public class ReactiveCouchbaseRepositoryKeyValueIntegrationTests extends Cluster

@Autowired ReactiveUserRepository userRepository;

@Autowired ReactiveAirportRepository airportRepository;
@Autowired ReactiveAirportRepository reactiveAirportRepository;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@mikereiche mikereiche merged commit 857a364 into 4.4.x Jun 17, 2022
@mikereiche mikereiche deleted the datacouch_1464_fix_1441_in_4_4_x_consider_scope_and_collection_in_repo_delete branch June 21, 2022 17:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants